fix(mobile): drop the dead /model row and re-park the caret on draft switch by kirill-markin · Pull Request #7523 · pingdotgg/t3code

kirill-markin · 2026-08-19T10:03:41Z

What this PR is now

#8587 landed the draft composer trigger menus, so the original 775-line version of this PR is obsolete. I have rebased onto main and cut it down to the two behaviors @juliusmarminge named when reopening it:

Reopening: #8587 added the draft menus, but this PR also suppresses the nonfunctional /model row on drafts and resets selection synchronously when drafts change. Those details are not in the merged implementation.

One file, +10 / −4, both edits in apps/mobile/src/features/threads/use-composer-command-menu.ts.

1. `/model` is a dead row on the new-task draft screen

allBuiltIn let /model bypass the capability gate, so it rendered on both surfaces. Selecting it on a draft does nothing useful: onSelect has no model branch, so it inserts the literal text /model , detectComposerTrigger then returns a slash-model trigger this hook builds no items for, and the menu just closes. The draft screen already has a working model control in the same toolbar ("Choose model"), so the row is redundant as well as inert.

The row is now gated on hasThread. ThreadComposer passes hasThread: true and is unchanged.

One consequence worth naming: with plan mode off, NewTaskDraftScreen passes no onUpdateInteractionMode, so the built-in list is now empty. Typing / mid-line on such a draft — where provider commands are position-gated to column 0 — can produce no menu at all, whereas before it always produced the inert /model row. I think no menu beats a row that does nothing, but say the word if you would rather keep something there.

2. The caret is re-parked a render too late

The ownerKey reset ran in a useEffect, which commits after the render that already computed trigger from the previous draft's caret offset. Switching drafts could therefore flash the popover open over a draft the user never typed a trigger into. It now adjusts during render, so that pass never reaches the screen. The ref is assigned before setSelection, so the restarted render cannot loop — the same shape already used in HomeScreen.tsx and ThreadNavigationSidebar.tsx.

No path search was ever dispatched in that window — useComposerPathSearch debounces its target by 200 ms — so this is a rendering fix only.

Deliberately out of scope

Making /model work on mobile. Nothing in the repo consumes the slash-model trigger kind that packages/shared/src/composerTrigger.ts produces; only apps/web's ChatComposer handles the model command, by opening its own picker. So the row is inert in ThreadComposer too, not just on drafts. Pre-existing and untouched here — it needs a real mobile model picker wired into the menu. Happy to take that as its own PR.
The items.length > 0 guard that hides ComposerCommandPopover's loading and empty states at both call sites. Relaxing it is a deliberate UX change to the most-used surface, so it stays its own PR.
New tests. apps/mobile has no hook-render harness — no renderHook, no testing-library — and use-composer-command-menu.test.ts covers only the pure composerSelectionAtEnd. Adding that harness, or extracting the built-in filter into a new module to assert one boolean, is more machinery than this change earns. Say the word if you would rather have it.

Verification

CI on this PR is the gate; I ran no local checks. Statically: hasThread was already in the items memo's dependency list, ThreadComposer.tsx passes hasThread: true so its menu is byte-for-byte unchanged, and the clamp effect on draftMessage.length is a no-op after the re-park. Happy to add an emulator pass with before/after images if you want them for this one.

Model: Claude Opus 5, harness: Claude Code.

coderabbitai · 2026-08-19T10:04:03Z

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6e1c202d-e188-4e9d-b193-9184b876d2e1

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

macroscopeapp · 2026-08-19T10:05:17Z

    !flow.submitting &&
    !(flow.workspaceMode === "worktree" && !flow.selectedBranchName);
+  // A share transfer locks the editor, so its rows must not be tappable.
+  const showComposerTriggerMenu =


🟡 Medium threads/NewTaskDraftScreen.tsx:847

When an @ search is loading or returns no matches, the trigger menu disappears without showing ComposerCommandPopover, so users receive no loading or empty-results feedback. The items.length > 0 guard makes the popover's isLoading and empty-state rendering unreachable; render it whenever a trigger is active.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/mobile/src/features/threads/NewTaskDraftScreen.tsx around line 847: When an `@` search is loading or returns no matches, the trigger menu disappears without showing `ComposerCommandPopover`, so users receive no loading or empty-results feedback. The `items.length > 0` guard makes the popover's `isLoading` and empty-state rendering unreachable; render it whenever a trigger is active.

Checked this against origin/main: the gate is pre-existing, not introduced here.

ThreadComposer.tsx:725 on main already reads composerTrigger && composerMenuItems.length > 0, so ComposerCommandPopover's empty-state branch — and the emptyText / "Searching files…" copy at ComposerCommandPopover.tsx:89-91,177 — was already unreachable on mobile before this PR. This patch keeps the same gate on both call sites so the two surfaces behave identically, which is what the "thread composer keeps its previous behavior exactly" constraint requires.

Relaxing it is a real improvement, but it is a deliberate UX change to both surfaces (a popover that appears on a bare @ and shows a spinner), so it belongs in its own PR rather than riding along here. Happy to send that follow-up if you want it — the one-line change is the items.length > 0 half of the condition at both call sites.

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

macroscopeapp · 2026-08-19T10:13:19Z

Approvability

Verdict: Would Approve

Macroscope's review found this PR approvable — This is a small, localized mobile bug fix: it synchronizes caret reset on composer switches and removes a nonfunctional /model row from unsent drafts while preserving existing thread behavior. A separate unresolved Medium finding concerns pre-existing missing loading/empty-state feedback for @ searches and should remain visible as a merge risk.

Not approved because:

1 blocking correctness issue found at or above your repo's Minimum Blocking Severity

^{Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.}

kirill-markin · 2026-08-19T10:42:29Z

Status, so this does not read as abandoned — the blocking finding is answered in the review thread: #7523 (comment)

Short version: the items.length > 0 guard that hides the popover's loading and empty state is not introduced here. ThreadComposer.tsx:725 already has it on main, so ComposerCommandPopover's emptyText branch (ComposerCommandPopover.tsx:89-91,177) was unreachable on mobile before this PR. Keeping the identical guard on both call sites is what makes the new surface behave exactly like the thread composer, which is the constraint this PR set for itself. Relaxing it is a deliberate UX change to both surfaces — a bare @ would start opening a popover with a spinner on the most-used surface — so I would rather send it as its own PR than bundle it here. Say the word and I will.

Everything else is green: Cursor Bugbot, Macroscope Effect Service Conventions, and the label checks. The Vercel marketing deploy fails with "Authorization required to deploy", which is the standard restriction on fork PRs rather than anything in this branch.

I also verified this locally instead of leaning on CI: 20/20 unit tests for the extracted logic, clean tsc --noEmit for apps/mobile, and a run on an Android emulator against a disposable backend where $ lists provider skills, @ lists project files, and tapping a row inserts the mention. Before/after images and a breakdown of the size:XL label are in the description.

juliusmarminge · 2026-09-01T23:23:10Z

Reopening: #8587 added the draft menus, but this PR also suppresses the nonfunctional /model row on drafts and resets selection synchronously when drafts change. Those details are not in the merged implementation. My initial closure missed that remaining scope.

…switch pingdotgg#8587 landed the draft composer trigger menus. Two details it left out. `/model` bypassed the built-in capability gate, so it rendered on the new-task draft screen, where selecting it only inserts the literal text `/model `: `onSelect` has no `model` branch, and the `slash-model` trigger that follows builds no items. It is now gated on `hasThread`, so the thread composer is unchanged and the draft screen, which already carries its own model control, loses the dead row. The `ownerKey` caret reset ran in an effect, one commit too late, so switching drafts could flash the trigger popover open over a draft the user never typed a trigger into. It now adjusts during render, with the ref assigned before `setSelection` so the restarted render cannot loop. Model: Claude Opus 5, harness: Claude Code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

kirill-markin · 2026-09-02T07:34:58Z

@juliusmarminge — rebased onto main and cut this down to exactly the two behaviors you named in the reopen. Force-pushed ce5d27c → 6c68b1c; the previous SHA is still in the timeline above if you want to compare.

The 775-line extraction is gone — #8587 owns that logic now, and re-landing my version of it would just be churn. What is left is one file, +10 / −4:

/model gated on hasThread, so it disappears from the new-task draft screen and ThreadComposer is untouched;
the ownerKey caret reset moved from a useEffect into the render body, so a draft switch can no longer flash the popover open for a frame.

Description rewritten to match. Two things in there worth your eye: with plan mode off, typing / mid-line on a draft can now produce no menu at all rather than the inert /model row, which I think is the right trade but is your call; and I skipped tests because apps/mobile has no hook-render harness — happy to add one if you would rather have the coverage.

kirill-markin mentioned this pull request Aug 19, 2026

[Bug]: Mobile new-task composer has no /, $ or @ picker — the first message of a thread cannot invoke a skill #7498

Closed

2 tasks

github-actions Bot added the vouch:unvouched PR author is not yet trusted in the VOUCHED list. label Aug 19, 2026

github-actions Bot added the size:XL 500-999 changed lines (additions + deletions). label Aug 19, 2026

macroscopeapp Bot reviewed Aug 19, 2026

View reviewed changes

juliusmarminge closed this Sep 1, 2026

juliusmarminge reopened this Sep 1, 2026

kirill-markin force-pushed the fix/mobile-new-task-composer-triggers branch from ce5d27c to 6c68b1c Compare September 2, 2026 07:15

kirill-markin changed the title ~~fix(mobile): support /, $ and @ in the new-task composer~~ fix(mobile): drop the dead /model row and re-park the caret on draft switch Sep 2, 2026

github-actions Bot added size:S 10-29 changed lines (additions + deletions). and removed size:XL 500-999 changed lines (additions + deletions). labels Sep 2, 2026

Navigation Menu

Uh oh!

fix(mobile): drop the dead /model row and re-park the caret on draft switch - #7523

fix(mobile): drop the dead /model row and re-park the caret on draft switch#7523
kirill-markin wants to merge 1 commit into
pingdotgg:mainfrom
kirill-markin:fix/mobile-new-task-composer-triggers

kirill-markin commented Aug 19, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Aug 19, 2026

Review skipped

Uh oh!

macroscopeapp Bot Aug 19, 2026

Uh oh!

kirill-markin Aug 19, 2026

Uh oh!

macroscopeapp Bot Aug 19, 2026

Uh oh!

macroscopeapp Bot commented Aug 19, 2026 •

edited

Loading

Uh oh!

kirill-markin commented Aug 19, 2026

Uh oh!

juliusmarminge commented Sep 1, 2026 •

edited

Loading

Uh oh!

kirill-markin commented Sep 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

kirill-markin commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR is now

1. /model is a dead row on the new-task draft screen

2. The caret is re-parked a render too late

Deliberately out of scope

Verification

Uh oh!

coderabbitai Bot commented Aug 19, 2026

Review skipped

Uh oh!

macroscopeapp Bot Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

kirill-markin Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Aug 19, 2026

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Approvability

Uh oh!

kirill-markin commented Aug 19, 2026

Uh oh!

juliusmarminge commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kirill-markin commented Sep 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kirill-markin commented Aug 19, 2026 •

edited

Loading

1. `/model` is a dead row on the new-task draft screen

macroscopeapp Bot commented Aug 19, 2026 •

edited

Loading

juliusmarminge commented Sep 1, 2026 •

edited

Loading